Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[docs-beta] rm comment in dockerfile as is invalid syntax #26272

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

cmpadden
Copy link
Contributor

@cmpadden cmpadden commented Dec 4, 2024

Summary & Motivation

It is invalid syntax to have a comment after a \ in your Dockerfile.

How I Tested These Changes

Ran locally.

Changelog

Insert changelog entry or delete this section.

@cmpadden cmpadden requested a review from neverett as a code owner December 4, 2024 15:45
@cmpadden
Copy link
Contributor Author

cmpadden commented Dec 4, 2024

It would be really nice if we had the Dagster.yaml or full example project referenced in this guide, similar to the existing Docker example:

https://github.com/dagster-io/dagster/tree/master/examples/deploy_docker

@cmpadden
Copy link
Contributor Author

cmpadden commented Dec 4, 2024

I've updated the deploy_docker example to set concurrency limits and use a definitions.py.

https://github.com/dagster-io/dagster/pull/26275/files

We should consolidate this guide, and this example.

Copy link
Contributor

@neverett neverett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just noting that this file is now in guides/deploy/deployment-locations/

@cmpadden cmpadden force-pushed the colton/fix-docker-tutorial branch from b61b835 to b335bfd Compare December 20, 2024 19:38
@cmpadden cmpadden merged commit 8fe1c12 into master Dec 20, 2024
2 of 3 checks passed
@cmpadden cmpadden deleted the colton/fix-docker-tutorial branch December 20, 2024 19:38
Copy link

Deploy preview for dagster-docs-beta ready!

Preview available at https://dagster-docs-beta-ctmpaumw0-elementl.vercel.app

Direct link to changed pages:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants